Skip to content

[FIX] [BACKPORT] [4.13] Rethrow takeVMSnapshot() exception#3761

Merged
yadvr merged 1 commit into
apache:4.13from
onitake:fix/4.13/snapshot-throw-not-null
Jan 28, 2020
Merged

[FIX] [BACKPORT] [4.13] Rethrow takeVMSnapshot() exception#3761
yadvr merged 1 commit into
apache:4.13from
onitake:fix/4.13/snapshot-throw-not-null

Conversation

@onitake
Copy link
Copy Markdown
Contributor

@onitake onitake commented Dec 11, 2019

Description

Backport of #3546 to 4.13: Rethrow takeVMSnapshot() exception instead of returning null in VMSnapshotManagerImpl.

Contributes to: #3518

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

Screenshots (if appropriate):

How Has This Been Tested?

The change has been tested on the master branch for 4.14. See #3546.

@yadvr yadvr added this to the 4.13.1.0 milestone Dec 12, 2019
GabrielBrascher
GabrielBrascher approved these changes Dec 12, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@GabrielBrascher GabrielBrascher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM

DaanHoogland
DaanHoogland reviewed Dec 13, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code is familiar and looks ok, but how did we test this in master?

@DaanHoogland
Copy link
Copy Markdown
Contributor

DaanHoogland commented Jan 21, 2020

@blueorangutan package

@apache apache deleted a comment from blueorangutan Jan 21, 2020
@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jan 21, 2020

@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@apache apache deleted a comment from blueorangutan Jan 21, 2020
@apache apache deleted a comment from yadvr Jan 21, 2020
@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jan 21, 2020

Packaging result: ✖centos6 ✔centos7 ✔debian. JID-640

@onitake
Copy link
Copy Markdown
Contributor Author

onitake commented Jan 21, 2020

Are the build errors errors relevant?
I see things like

CloudstackAPIException: Execute cmd: listprojectaccounts failed, due to: errorCode: 431, errorText:Unable to find the project id=9

which doesn't look related to me at all...

@DaanHoogland
Copy link
Copy Markdown
Contributor

DaanHoogland commented Jan 22, 2020

@onitake where do you see those errors?

@DaanHoogland
Copy link
Copy Markdown
Contributor

DaanHoogland commented Jan 22, 2020

@blueorangutan test

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jan 22, 2020

@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@onitake
Copy link
Copy Markdown
Contributor Author

onitake commented Jan 22, 2020

@DaanHoogland They were in the Travis build logs linked here, which was still in "failed" state yesterday. Looks like they were from a different branch or older build?
Or maybe it was simply an intermittent failure.

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jan 23, 2020

Trillian test result (tid-809)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 29643 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3761-t809-kvm-centos7.zip
Smoke tests completed. 77 look OK, 0 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File

@DaanHoogland
Copy link
Copy Markdown
Contributor

DaanHoogland commented Jan 23, 2020

@DaanHoogland They were in the Travis build logs linked here, which was still in "failed" state yesterday. Looks like they were from a different branch or older build?
Or maybe it was simply an intermittent failure.

maybe, i did re-kick travis, so... Not nice to not be able to rely on CI fully, but this is the complex eco system for you.

@yadvr yadvr merged commit 8792070 into apache:4.13 Jan 28, 2020
yadvr
yadvr approved these changes Jan 28, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@yadvr yadvr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, this is just a backported change.

@onitake onitake deleted the fix/4.13/snapshot-throw-not-null branch July 15, 2021 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DaanHoogland DaanHoogland DaanHoogland left review comments

@yadvr yadvr yadvr approved these changes

@GabrielBrascher GabrielBrascher GabrielBrascher approved these changes

Assignees

No one assigned

Labels

component:management-server type:bug

Projects

None yet

Milestone

4.13.1.0

Development

Successfully merging this pull request may close these issues.

7 participants

@onitake @DaanHoogland @blueorangutan @yadvr @GabrielBrascher @svenvogel @srgoni